Make upgradable #3
Conversation
| mapping(address => bool) private _admins; | ||
|
|
||
| constructor() { | ||
| function initializeAdministration() public virtual initializer { |
There was a problem hiding this comment.
あいまいな記憶で申し訳ないのですが、Upgradeableのinitialize系の関数名には、先頭に __ （アンダースコア２つ）をつけるという慣習があったような気がしています。
（ initializeAdministration() のようなイメージ）
違ったらすみません…
関数修飾子が internal だと安心かもです。
There was a problem hiding this comment.
Openzeppelinのドキュメントとかは普通にinitialize()ですね、もし参考になるものがあったら言ってください！
ただ、InteractCommunityToken.solとAdministration.solのinitializeはinternalのほうが良さそうなので、_つけるようにします
| communityToken = _communityToken; | ||
| } | ||
|
|
||
| function transferCommunityToken(uint256 _amount, address _to) internal { |
There was a problem hiding this comment.
_transferCommunityToken() はいかがでしょうか？
There was a problem hiding this comment.
この関数が今使われてないようなので、いったん消しました
| require(sent, "Ticket: ERC20 token transfer failed"); | ||
| } | ||
|
|
||
| function batchTransferCommunityToken(uint256 totalPrice, uint256[] memory _amounts, address[] memory _to) internal { |
There was a problem hiding this comment.
_batchTransferCommunityToken() はいかがでしょうか？
totalPrice が totalAmount だと揃ってて分かりやすいかもです。
| import "./interfaces/IHenkakuToken.sol"; | ||
|
|
||
| abstract contract InteractCommunityToken is Administration, MintManager { | ||
| address public communityToken; |
There was a problem hiding this comment.
address public communityToken; の値をAdminが変更できる関数を追加するのはいかがでしょうか？
There was a problem hiding this comment.
今回のアップデートとは関係のないところで、Ticket.sol の32～34行目について相談させてください！
以前とにかくガス代を下げたかったため私が uint64 を設定したのですが、他コミュニティに使用してもらうことを考えると、念のため3つとも uint256 に戻した方が良いかもと思いました。
いかがでしょうか？
There was a problem hiding this comment.
64でもかなりでかいのでこれで良さそうです
There was a problem hiding this comment.
こちらも今回のアプデとは関係なく、45～49行目の関数へのコメントは削除するのはいかがでしょうか？
There was a problem hiding this comment.
26～27行目の
mapping(address => uint256[]) private ownerOfRegisteredIds;
mapping(address => uint256[]) private ownerOfMintedIds;
ですが、private のためアンダースコアを追加するのはいかがでしょうか？
There was a problem hiding this comment.
関数内で_つかっている同じ名前の変数があるのでこのままにしておきます
contracts/community/Ticket.sol
Outdated
| @@ -53,20 +61,19 @@ contract Ticket is ERC1155, ERC1155Supply, Administration, MintManager, Interact | |||
|
|
|||
| TicketInfo[] private registeredTickets; | |||
There was a problem hiding this comment.
private のため _registeredTickets に変更するのはいかがでしょうか？
| import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; | ||
| import "./Administration.sol"; | ||
| import "./MintManager.sol"; | ||
| import "./interfaces/IHenkakuToken.sol"; |
There was a problem hiding this comment.
IHenkakuToken.sol を ICommunityToken.sol に変更するのはいかがでしょうか？
| Administration, | ||
| MintManager, | ||
| InteractCommunityToken | ||
| { |
There was a problem hiding this comment.
アップグレーダブルのパターンとしてUUPSを採用するのはいかがでしょうか？
ロジック内に誰がアップグレードできるかを決定する関数（ _authorizeUpgrade() ）を置くアブストラクトで、そちらを override することで modifier などで簡単にアップグレード権限を設定でき、 _authorizeUpgrade() 自体もアップグレードできるらしいと聞きました！
やったこと
#1